Module metadata: Fix stale module detection and add per-type metadata index#21231
Open
bcoles wants to merge 1 commit intorapid7:masterfrom
Open
Module metadata: Fix stale module detection and add per-type metadata index#21231bcoles wants to merge 1 commit intorapid7:masterfrom
bcoles wants to merge 1 commit intorapid7:masterfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes stale module detection in Msf::Modules::Metadata::Cache and introduces a per-type metadata index to avoid repeated full-cache scans when retrieving metadata by module type, plus a small load-path filtering optimization in the module manager cache.
Changes:
- Fix stale module detection when a module’s type changes but its path stays the same.
- Add
@module_metadata_by_typeindex and rebuild/update logic to speed upmodule_metadata(type)lookups. - Optimize allowed path checks by switching from
select(...).empty?toany?+start_with?.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| lib/msf/core/modules/metadata/cache.rb | Fixes stale-type deletion bug and adds per-type metadata index used by module_metadata(type). |
| lib/msf/core/module_manager/cache.rb | Avoids temporary array allocation when filtering metadata entries by allowed paths. |
41e43d5 to
780789c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes two bugs in the metadata cache and adds a per-type index to eliminate an O(n) scan that runs on every
module_metadata(type)call.Bug fixes in cache.rb
1. Stale module detection was a no-op
In
refresh_metadata_instance_internal, thedelete_ifblock comparedmodule_metadata.type != module_metadata.type(same object, both sides), which is always false. This meant modules that changed type (e.g. Auxiliary reclassified as Exploit) would leave stale entries in the cache. Fixed to compare againstmetadata_obj.type.2.
delete_ifblock relied on implicit return of&&The
delete_ifblock returned the result ofpath.eql?(…) && type != type. When both conditions are true,&&returns the right-hand operand — in this case a truthy string comparison result, which happened to work, but was fragile and unclear. Replaced with an explicitis_staleboolean variable.Performance: per-type metadata index
module_metadata(type)previously ranfilter_mapover all ~6,600 entries on every call (~14ms each). This is called once per module type during startup and tab-completion. Replaced with a pre-built@module_metadata_by_typehash:refresh_metadataoperationsMeasured improvement: ~14ms → ~0.001ms per call.
Minor cleanup in cache.rb
Replaced
allowed_paths.select{|x| path.index(x) == 0}.empty?withallowed_paths.any? { |x| path.start_with?(x) }to avoid allocating a temporary array on every loop iteration.Simplified
get_cache_keyReplaced multi-line string concatenation with string interpolation.